Make it obvious that the flag is controlling RNG schema validation. --- src/bhyve/bhyve_driver.c | 4 ++-- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 2 +- src/esx/esx_driver.c | 2 +- src/libxl/libxl_driver.c | 4 ++-- src/lxc/lxc_driver.c | 4 ++-- src/openvz/openvz_driver.c | 4 ++-- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/test/test_driver.c | 4 ++-- src/uml/uml_driver.c | 4 ++-- src/vbox/vbox_common.c | 2 +- src/vmware/vmware_driver.c | 4 ++-- src/vz/vz_driver.c | 2 +- src/xen/xen_driver.c | 4 ++-- src/xenapi/xenapi_driver.c | 4 ++-- 16 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index c4051a1..c15725f 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -526,7 +526,7 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; caps = bhyveDriverGetCapabilities(privconn); if (!caps) @@ -924,7 +924,7 @@ bhyveDomainCreateXML(virConnectPtr conn, VIR_DOMAIN_START_VALIDATE, NULL); if (flags & VIR_DOMAIN_START_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; if (flags & VIR_DOMAIN_START_AUTODESTROY) start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 568c699..e2e247a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15343,7 +15343,7 @@ virDomainDefParseXML(xmlDocPtr xml, bool usb_master = false; char *netprefix = NULL; - if (flags & VIR_DOMAIN_DEF_PARSE_VALIDATE) { + if (flags & VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA) { char *schema = virFileFindResource("domain.rng", abs_topsrcdir "/docs/schemas", PKGDATADIR "/schemas"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b1953b3..0d3b1e0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2575,7 +2575,7 @@ typedef enum { /* parse only source half of <disk> */ VIR_DOMAIN_DEF_PARSE_DISK_SOURCE = 1 << 6, /* perform RNG schema validation on the passed XML document */ - VIR_DOMAIN_DEF_PARSE_VALIDATE = 1 << 7, + VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA = 1 << 7, /* don't validate os.type and arch against capabilities. Prevents * VMs from disappearing when qemu is removed and libvirtd is restarted */ VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS = 1 << 8, diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 031c666..44cdf71 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3036,7 +3036,7 @@ esxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; memset(&data, 0, sizeof(data)); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 95dfc01..6086717 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -967,7 +967,7 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, VIR_DOMAIN_START_VALIDATE, NULL); if (flags & VIR_DOMAIN_START_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt, parse_flags))) @@ -2714,7 +2714,7 @@ libxlDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt, parse_flags))) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 9e03f1f..05dbf3a 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -463,7 +463,7 @@ lxcDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; if (!(caps = virLXCDriverGetCapabilities(driver, false))) goto cleanup; @@ -1221,7 +1221,7 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, if (flags & VIR_DOMAIN_START_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; virNWFilterReadLockFilterUpdates(); diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index a7474ff..73322c3 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -989,7 +989,7 @@ openvzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; openvzDriverLock(driver); if ((vmdef = virDomainDefParseString(xml, driver->caps, driver->xmlopt, @@ -1078,7 +1078,7 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); if (flags & VIR_DOMAIN_START_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; openvzDriverLock(driver); if ((vmdef = virDomainDefParseString(xml, driver->caps, driver->xmlopt, diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index da87686..cad51e7 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3569,7 +3569,7 @@ phypDomainCreateXML(virConnectPtr conn, virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); if (flags & VIR_DOMAIN_START_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; if (!(def = virDomainDefParseString(xml, phyp_driver->caps, phyp_driver->xmlopt, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 10d3e3d..0fbdee6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1780,7 +1780,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, VIR_DOMAIN_START_VALIDATE, NULL); if (flags & VIR_DOMAIN_START_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; if (flags & VIR_DOMAIN_START_PAUSED) start_flags |= VIR_QEMU_PROCESS_START_PAUSED; if (flags & VIR_DOMAIN_START_AUTODESTROY) @@ -7252,7 +7252,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; cfg = virQEMUDriverGetConfig(driver); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a51eb09..1eb7813 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1605,7 +1605,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); if (flags & VIR_DOMAIN_START_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; testDriverLock(privconn); if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, @@ -2610,7 +2610,7 @@ static virDomainPtr testDomainDefineXMLFlags(virConnectPtr conn, virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, parse_flags)) == NULL) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 923c3f6..df0e137 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1595,7 +1595,7 @@ static virDomainPtr umlDomainCreateXML(virConnectPtr conn, const char *xml, VIR_DOMAIN_START_VALIDATE, NULL); if (flags & VIR_DOMAIN_START_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; virNWFilterReadLockFilterUpdates(); umlDriverLock(driver); @@ -2077,7 +2077,7 @@ umlDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; umlDriverLock(driver); if (!(def = virDomainDefParseString(xml, driver->caps, driver->xmlopt, diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index ee2b6ad..b62e654 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1864,7 +1864,7 @@ vboxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; if (!data->vboxObj) return ret; diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 93f21c9..4ac2799 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -377,7 +377,7 @@ vmwareDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; ctx.parseFileName = NULL; ctx.formatFileName = vmwareCopyVMXFileName; @@ -670,7 +670,7 @@ vmwareDomainCreateXML(virConnectPtr conn, const char *xml, virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); if (flags & VIR_DOMAIN_START_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; ctx.parseFileName = NULL; ctx.formatFileName = vmwareCopyVMXFileName; diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 177a57a..a092689 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -742,7 +742,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; virObjectLock(driver); if ((def = virDomainDefParseString(xml, driver->caps, driver->xmlopt, diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 3f5d80d..db8f74e 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -759,7 +759,7 @@ xenUnifiedDomainCreateXML(virConnectPtr conn, virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); if (flags & VIR_DOMAIN_START_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; if (!(def = virDomainDefParseString(xml, priv->caps, priv->xmlopt, parse_flags))) @@ -1799,7 +1799,7 @@ xenUnifiedDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; if (!(def = virDomainDefParseString(xml, priv->caps, priv->xmlopt, parse_flags))) diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 97a1ada..070e8d2 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -562,7 +562,7 @@ xenapiDomainCreateXML(virConnectPtr conn, virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); if (flags & VIR_DOMAIN_START_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; virDomainDefPtr defPtr = virDomainDefParseString(xmlDesc, priv->caps, priv->xmlopt, @@ -1744,7 +1744,7 @@ xenapiDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; if (!priv->caps) return NULL; -- 2.8.3

Until now we weren't able to add checks would reject configuration once accepted by the parser. This patch adds a new callback and infrastructure to add such checks. In this patch all the places where rejecting a now-invalid configuration wouldn't be a good idea are marked with a new parser flag. --- src/conf/domain_conf.c | 48 ++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 16 +++++++++++++++ src/conf/snapshot_conf.c | 3 ++- src/conf/virdomainobjlist.c | 6 ++++-- src/libvirt_private.syms | 1 + src/libxl/libxl_domain.c | 3 ++- src/libxl/libxl_migration.c | 6 ++++-- src/openvz/openvz_driver.c | 3 ++- src/qemu/qemu_domain.c | 3 ++- src/qemu/qemu_driver.c | 9 +++++--- src/qemu/qemu_migration.c | 12 +++++++---- src/security/virt-aa-helper.c | 3 ++- 12 files changed, 96 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e2e247a..d6bd737 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4540,6 +4540,47 @@ virDomainDefPostParse(virDomainDefPtr def, } +static int +virDomainDefValidateInternal(const virDomainDef *def ATTRIBUTE_UNUSED) +{ + return 0; +} + + +/** + * virDomainDefValidate: + * @def: domain definition + * + * This validation function is designed to take checks of globally invalid + * configurations that the parser needs to accept so that VMs don't vanish upon + * daemon restart. Such definition can be rejected upon startup or define, where + * this function shall be called. + * + * Returns 0 if domain definition is valid, -1 on error and reports an + * appropriate message. + */ +int +virDomainDefValidate(const virDomainDef *def, + virCapsPtr caps, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt) +{ + /* validate configuration only in certain places */ + if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE) + return 0; + + /* call the domain config callback */ + if (xmlopt->config.domainValidateCallback && + xmlopt->config.domainValidateCallback(def, caps, xmlopt->config.priv) < 0) + return -1; + + if (virDomainDefValidateInternal(def) < 0) + return -1; + + return 0; +} + + void virDomainDefClearPCIAddresses(virDomainDefPtr def) { virDomainDeviceInfoIterate(def, virDomainDeviceInfoClearPCIAddress, NULL); @@ -16951,6 +16992,10 @@ virDomainDefParseXML(xmlDocPtr xml, if (virDomainDefPostParse(def, caps, flags, xmlopt) < 0) goto error; + /* valdiate configuration */ + if (virDomainDefValidate(def, caps, flags, xmlopt) < 0) + goto error; + virHashFree(bootHash); return def; @@ -23720,7 +23765,8 @@ virDomainDefCopy(virDomainDefPtr src, char *xml; virDomainDefPtr ret; unsigned int format_flags = VIR_DOMAIN_DEF_FORMAT_SECURE; - unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; if (migratable) format_flags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE | VIR_DOMAIN_DEF_FORMAT_MIGRATABLE; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0d3b1e0..9c940b6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2361,6 +2361,12 @@ typedef int (*virDomainDefAssignAddressesCallback)(virDomainDef *def, unsigned int parseFlags, void *opaque); +/* Called in appropriate places where the domain conf parser can return failure + * for configurations that were previously accepted. This shall not modify the + * config. */ +typedef int (*virDomainDefValidateCallback)(const virDomainDef *def, + virCapsPtr caps, + void *opaque); typedef struct _virDomainDefParserConfig virDomainDefParserConfig; typedef virDomainDefParserConfig *virDomainDefParserConfigPtr; @@ -2370,6 +2376,9 @@ struct _virDomainDefParserConfig { virDomainDeviceDefPostParseCallback devicesPostParseCallback; virDomainDefAssignAddressesCallback assignAddressesCallback; + /* validation callbacks */ + virDomainDefValidateCallback domainValidateCallback; + /* private data for the callbacks */ void *priv; virFreeCallback privFree; @@ -2415,6 +2424,11 @@ virDomainDefPostParse(virDomainDefPtr def, unsigned int parseFlags, virDomainXMLOptionPtr xmlopt); +int virDomainDefValidate(const virDomainDef *def, + virCapsPtr caps, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt); + static inline bool virDomainObjIsActive(virDomainObjPtr dom) { @@ -2581,6 +2595,8 @@ typedef enum { VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS = 1 << 8, /* allow updates in post parse callback that would break ABI otherwise */ VIR_DOMAIN_DEF_PARSE_ABI_UPDATE = 1 << 9, + /* skip definition validation checks meant to be executed on define time only */ + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE = 1 << 10, } virDomainDefParseFlags; typedef enum { diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index c7794e9..c52c481 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -269,7 +269,8 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, * clients will have to decide between best effort * initialization or outright failure. */ if ((tmp = virXPathString("string(./domain/@type)", ctxt))) { - int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE; + int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL) domainflags |= VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS; xmlNodePtr domainNode = virXPathNode("./domain", ctxt); diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 05c532a..27590c8 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -443,7 +443,8 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms, goto error; if (!(def = virDomainDefParseFile(configFile, caps, xmlopt, VIR_DOMAIN_DEF_PARSE_INACTIVE | - VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS))) + VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto error; if ((autostartLink = virDomainConfigFile(autostartDir, name)) == NULL) @@ -493,7 +494,8 @@ virDomainObjListLoadStatus(virDomainObjListPtr doms, VIR_DOMAIN_DEF_PARSE_STATUS | VIR_DOMAIN_DEF_PARSE_ACTUAL_NET | VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES | - VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS))) + VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto error; virUUIDFormat(obj->def->uuid, uuidstr); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e325168..9b9bfab 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -244,6 +244,7 @@ virDomainDefSetMemoryInitial; virDomainDefSetMemoryTotal; virDomainDefSetVcpus; virDomainDefSetVcpusMax; +virDomainDefValidate; virDomainDeleteConfig; virDomainDeviceAddressIsValid; virDomainDeviceAddressTypeToString; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 8a3866f..9a3fe6a 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -683,7 +683,8 @@ libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver, } if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto error; VIR_FREE(xml); diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index a809aa0..539ee1b 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -419,7 +419,8 @@ libxlDomainMigrationBegin(virConnectPtr conn, if (xmlin) { if (!(tmpdef = virDomainDefParseString(xmlin, cfg->caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto endjob; if (!libxlDomainDefCheckABIStability(driver, vm->def, tmpdef)) @@ -465,7 +466,8 @@ libxlDomainMigrationPrepareDef(libxlDriverPrivatePtr driver, } if (!(def = virDomainDefParseString(dom_xml, cfg->caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto cleanup; if (dname) { diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 73322c3..aa482c9 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -2315,7 +2315,8 @@ openvzDomainMigratePrepare3Params(virConnectPtr dconn, } if (!(def = virDomainDefParseString(dom_xml, driver->caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto error; if (!(vm = virDomainObjListAdd(driver->domains, def, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c21465d..fd6d6d2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2990,7 +2990,8 @@ qemuDomainDefCopy(virQEMUDriverPtr driver, goto cleanup; if (!(ret = virDomainDefParseString(xml, caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto cleanup; cleanup: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0fbdee6..8463e8d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3227,7 +3227,8 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, virDomainDefPtr def = NULL; if (!(def = virDomainDefParseString(xmlin, caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) { + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) { goto endjob; } if (!qemuDomainDefCheckABIStability(driver, vm->def, def)) { @@ -6414,7 +6415,8 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, /* Create a domain from this XML */ if (!(def = virDomainDefParseString(xml, caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto error; if (xmlout) @@ -14481,7 +14483,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, * conversion in and back out of xml. */ if (!(xml = qemuDomainDefFormatLive(driver, vm->def, true, true)) || !(def->dom = virDomainDefParseString(xml, caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto endjob; if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c5b2963..c2749d4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1279,7 +1279,8 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, mig->persistent = virDomainDefParseNode(doc, nodes[0], caps, driver->xmlopt, VIR_DOMAIN_DEF_PARSE_INACTIVE | - VIR_DOMAIN_DEF_PARSE_ABI_UPDATE); + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); if (!mig->persistent) { /* virDomainDefParseNode already reported * an error for us */ @@ -3208,7 +3209,8 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, if (xmlin) { if (!(def = virDomainDefParseString(xmlin, caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto cleanup; if (!qemuDomainDefCheckABIStability(driver, vm->def, def)) @@ -3551,7 +3553,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, VIR_DEBUG("Using hook-filtered domain XML: %s", xmlout); newdef = virDomainDefParseString(xmlout, caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE); + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); if (!newdef) goto cleanup; @@ -4024,7 +4027,8 @@ qemuMigrationPrepareDef(virQEMUDriverPtr driver, return NULL; if (!(def = virDomainDefParseString(dom_xml, caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto cleanup; if (dname) { diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 691bbdf..6b0685c 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -704,7 +704,8 @@ get_definition(vahControl * ctl, const char *xmlStr) } ctl->def = virDomainDefParseString(xmlStr, - ctl->caps, ctl->xmlopt, 0); + ctl->caps, ctl->xmlopt, + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); if (ctl->def == NULL) { vah_error(ctl, 0, _("could not parse XML")); -- 2.8.3

It's passed to all places along with the structure. --- src/conf/domain_conf.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d6bd737..46820c2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4420,7 +4420,6 @@ virDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, struct virDomainDefPostParseDeviceIteratorData { - virDomainDefPtr def; virCapsPtr caps; virDomainXMLOptionPtr xmlopt; unsigned int parseFlags; @@ -4428,13 +4427,13 @@ struct virDomainDefPostParseDeviceIteratorData { static int -virDomainDefPostParseDeviceIterator(virDomainDefPtr def ATTRIBUTE_UNUSED, +virDomainDefPostParseDeviceIterator(virDomainDefPtr def, virDomainDeviceDefPtr dev, virDomainDeviceInfoPtr info ATTRIBUTE_UNUSED, void *opaque) { struct virDomainDefPostParseDeviceIteratorData *data = opaque; - return virDomainDeviceDefPostParse(dev, data->def, data->caps, + return virDomainDeviceDefPostParse(dev, def, data->caps, data->parseFlags, data->xmlopt); } @@ -4477,7 +4476,7 @@ virDomainDefPostParseInternal(virDomainDefPtr def, /* videos[0] might have been added in AddImplicitDevices, after we've * done the per-device post-parse */ - if (virDomainDefPostParseDeviceIterator(NULL, &device, NULL, data) < 0) + if (virDomainDefPostParseDeviceIterator(def, &device, NULL, data) < 0) return -1; } @@ -4496,7 +4495,6 @@ virDomainDefPostParse(virDomainDefPtr def, { int ret; struct virDomainDefPostParseDeviceIteratorData data = { - .def = def, .caps = caps, .xmlopt = xmlopt, .parseFlags = parseFlags, -- 2.8.3

Similarly to the domain definition validator add a device validator. The change to the prototype of the domain validator is necessary as virDomainDeviceInfoIterateInternal requires a non-const pointer. --- src/conf/domain_conf.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++-- src/conf/domain_conf.h | 9 ++++++- src/libxl/libxl_driver.c | 6 +++-- src/lxc/lxc_driver.c | 3 ++- src/qemu/qemu_driver.c | 2 +- src/uml/uml_driver.c | 3 ++- src/vbox/vbox_common.c | 3 ++- src/vz/vz_driver.c | 4 +++- src/xen/xend_internal.c | 3 ++- src/xen/xm_internal.c | 3 ++- 10 files changed, 86 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 46820c2..b8a6901 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4539,6 +4539,47 @@ virDomainDefPostParse(virDomainDefPtr def, static int +virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev ATTRIBUTE_UNUSED, + const virDomainDef *def ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static int +virDomainDeviceDefValidate(const virDomainDeviceDef *dev, + const virDomainDef *def, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt) +{ + /* validate configuration only in certain places */ + if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE) + return 0; + + if (xmlopt->config.deviceValidateCallback && + xmlopt->config.deviceValidateCallback(dev, def, xmlopt->config.priv)) + return -1; + + if (virDomainDeviceDefValidateInternal(dev, def) < 0) + return -1; + + return 0; +} + + +static int +virDomainDefValidateDeviceIterator(virDomainDefPtr def, + virDomainDeviceDefPtr dev, + virDomainDeviceInfoPtr info ATTRIBUTE_UNUSED, + void *opaque) +{ + struct virDomainDefPostParseDeviceIteratorData *data = opaque; + return virDomainDeviceDefValidate(dev, def, + data->parseFlags, data->xmlopt); +} + + +static int virDomainDefValidateInternal(const virDomainDef *def ATTRIBUTE_UNUSED) { return 0; @@ -4558,11 +4599,17 @@ virDomainDefValidateInternal(const virDomainDef *def ATTRIBUTE_UNUSED) * appropriate message. */ int -virDomainDefValidate(const virDomainDef *def, +virDomainDefValidate(virDomainDefPtr def, virCapsPtr caps, unsigned int parseFlags, virDomainXMLOptionPtr xmlopt) { + struct virDomainDefPostParseDeviceIteratorData data = { + .caps = caps, + .xmlopt = xmlopt, + .parseFlags = parseFlags, + }; + /* validate configuration only in certain places */ if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE) return 0; @@ -4572,6 +4619,12 @@ virDomainDefValidate(const virDomainDef *def, xmlopt->config.domainValidateCallback(def, caps, xmlopt->config.priv) < 0) return -1; + /* iterate the devices */ + if (virDomainDeviceInfoIterateInternal(def, + virDomainDefValidateDeviceIterator, + true, &data) < 0) + return -1; + if (virDomainDefValidateInternal(def) < 0) return -1; @@ -13183,6 +13236,10 @@ virDomainDeviceDefParse(const char *xmlStr, if (virDomainDeviceDefPostParse(dev, def, caps, flags, xmlopt) < 0) goto error; + /* validate the configuration */ + if (virDomainDeviceDefValidate(dev, def, flags, xmlopt) < 0) + goto error; + cleanup: xmlFreeDoc(xml); xmlXPathFreeContext(ctxt); @@ -24222,7 +24279,8 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, xmlStr = virBufferContentAndReset(&buf); ret = virDomainDeviceDefParse(xmlStr, def, caps, xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE); + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); cleanup: VIR_FREE(xmlStr); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9c940b6..215235f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2368,6 +2368,12 @@ typedef int (*virDomainDefValidateCallback)(const virDomainDef *def, virCapsPtr caps, void *opaque); +/* Called once per device, for adjusting per-device settings while + * leaving the overall domain otherwise unchanged. */ +typedef int (*virDomainDeviceDefValidateCallback)(const virDomainDeviceDef *dev, + const virDomainDef *def, + void *opaque); + typedef struct _virDomainDefParserConfig virDomainDefParserConfig; typedef virDomainDefParserConfig *virDomainDefParserConfigPtr; struct _virDomainDefParserConfig { @@ -2378,6 +2384,7 @@ struct _virDomainDefParserConfig { /* validation callbacks */ virDomainDefValidateCallback domainValidateCallback; + virDomainDeviceDefValidateCallback deviceValidateCallback; /* private data for the callbacks */ void *priv; @@ -2424,7 +2431,7 @@ virDomainDefPostParse(virDomainDefPtr def, unsigned int parseFlags, virDomainXMLOptionPtr xmlopt); -int virDomainDefValidate(const virDomainDef *def, +int virDomainDefValidate(virDomainDefPtr def, virCapsPtr caps, unsigned int parseFlags, virDomainXMLOptionPtr xmlopt); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 6086717..bbcf8b9 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3742,7 +3742,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto endjob; /* Make a copy for updated domain. */ @@ -3759,7 +3760,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, virDomainDeviceDefFree(dev); if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto endjob; if (libxlDomainDetachDeviceLive(driver, vm, dev) < 0) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 05dbf3a..69cfa78 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5315,7 +5315,8 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom, dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE); + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); if (dev == NULL) goto endjob; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8463e8d..a7202ed 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8422,7 +8422,7 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom, virDomainDefPtr vmdef = NULL; virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; int ret = -1; - unsigned int parse_flags = 0; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; virQEMUCapsPtr qemuCaps = NULL; qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index df0e137..3d25135 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2366,7 +2366,8 @@ static int umlDomainDetachDevice(virDomainPtr dom, const char *xml) } dev = virDomainDeviceDefParse(xml, vm->def, driver->caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE); + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); if (dev == NULL) goto cleanup; diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index b62e654..ad5cbe9 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -4242,7 +4242,8 @@ static int vboxDomainDetachDevice(virDomainPtr dom, const char *xml) def->os.type = VIR_DOMAIN_OSTYPE_HVM; dev = virDomainDeviceDefParse(xml, def, data->caps, data->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE); + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); if (dev == NULL) goto cleanup; diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index a092689..e59cf30 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1211,7 +1211,9 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, goto cleanup; dev = virDomainDeviceDefParse(xml, privdom->def, privconn->driver->caps, - privconn->driver->xmlopt, VIR_DOMAIN_XML_INACTIVE); + privconn->driver->xmlopt, + VIR_DOMAIN_XML_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); if (dev == NULL) goto cleanup; diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 4dd6b41..3e3be58 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2355,7 +2355,8 @@ xenDaemonDetachDeviceFlags(virConnectPtr conn, goto cleanup; if (!(dev = virDomainDeviceDefParse(xml, def, priv->caps, priv->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto cleanup; if (virDomainXMLDevID(conn, minidef, dev, class, ref, sizeof(ref))) diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index ef1a460..4c9dcdf 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1343,7 +1343,8 @@ xenXMDomainDetachDeviceFlags(virConnectPtr conn, if (!(dev = virDomainDeviceDefParse(xml, entry->def, priv->caps, priv->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto cleanup; switch (dev->type) { -- 2.8.3

--- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_process.h | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e847cd1..a5bb99e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4325,7 +4325,7 @@ qemuProcessStartValidateXML(virDomainObjPtr vm, * start the domain but create a valid qemu command. If some code shouldn't be * executed in this case, make sure to check this flag. */ -int +static int qemuProcessStartValidate(virQEMUDriverPtr driver, virDomainObjPtr vm, virQEMUCapsPtr qemuCaps, diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 61d9f56..13845d7 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -89,13 +89,6 @@ virCommandPtr qemuProcessCreatePretendCmd(virConnectPtr conn, bool standalone, unsigned int flags); -int qemuProcessStartValidate(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virQEMUCapsPtr qemuCaps, - bool migration, - bool snap, - unsigned int flags); - int qemuProcessInit(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, -- 2.8.3

Validation of qemu process startup requires to know whether the process is used for a fresh VM or whether it's reloaded from a snapshot/migration. Pass this information in via a flag rather than calculating it from a bunch of bools. --- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_process.c | 26 +++++++++++++------------- src/qemu/qemu_process.h | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c2749d4..aad7284 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3628,7 +3628,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, } if (qemuProcessInit(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN, - true, false, VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) + true, VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) goto stopjob; stopProcess = true; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a5bb99e..f8afb36 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4285,8 +4285,7 @@ qemuProcessStartWarnShmem(virDomainObjPtr vm) static int qemuProcessStartValidateXML(virDomainObjPtr vm, virQEMUCapsPtr qemuCaps, - bool migration, - bool snapshot) + unsigned int flags) { /* The bits we validate here are XML configs that we previously * accepted. We reject them at VM startup time rather than parse @@ -4299,7 +4298,10 @@ qemuProcessStartValidateXML(virDomainObjPtr vm, if (qemuValidateCpuCount(vm->def, qemuCaps) < 0) return -1; - if (!migration && !snapshot && + /* checks below should not be executed when starting a qemu process for a + * VM that was running before (migration, snapshots, save). It's more + * important to start such VM than keep the configuration clean */ + if ((flags & VIR_QEMU_PROCESS_START_NEW) && virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) return -1; @@ -4329,8 +4331,6 @@ static int qemuProcessStartValidate(virQEMUDriverPtr driver, virDomainObjPtr vm, virQEMUCapsPtr qemuCaps, - bool migration, - bool snapshot, unsigned int flags) { size_t i; @@ -4358,7 +4358,7 @@ qemuProcessStartValidate(virQEMUDriverPtr driver, } - if (qemuProcessStartValidateXML(vm, qemuCaps, migration, snapshot) < 0) + if (qemuProcessStartValidateXML(vm, qemuCaps, flags) < 0) return -1; VIR_DEBUG("Checking for any possible (non-fatal) issues"); @@ -4408,7 +4408,6 @@ qemuProcessInit(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, bool migration, - bool snap, unsigned int flags) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); @@ -4438,8 +4437,7 @@ qemuProcessInit(virQEMUDriverPtr driver, vm->def->os.machine))) goto cleanup; - if (qemuProcessStartValidate(driver, vm, priv->qemuCaps, - migration, snap, flags) < 0) + if (qemuProcessStartValidate(driver, vm, priv->qemuCaps, flags) < 0) goto cleanup; /* Do this upfront, so any part of the startup process can add @@ -5415,8 +5413,10 @@ qemuProcessStart(virConnectPtr conn, VIR_QEMU_PROCESS_START_PAUSED | VIR_QEMU_PROCESS_START_AUTODESTROY, cleanup); - if (qemuProcessInit(driver, vm, asyncJob, !!migrateFrom, - !!snapshot, flags) < 0) + if (!migrateFrom && !snapshot) + flags |= VIR_QEMU_PROCESS_START_NEW; + + if (qemuProcessInit(driver, vm, asyncJob, !!migrateFrom, flags) < 0) goto cleanup; if (migrateFrom) { @@ -5493,9 +5493,9 @@ qemuProcessCreatePretendCmd(virConnectPtr conn, VIR_QEMU_PROCESS_START_AUTODESTROY, cleanup); flags |= VIR_QEMU_PROCESS_START_PRETEND; + flags |= VIR_QEMU_PROCESS_START_NEW; - if (qemuProcessInit(driver, vm, QEMU_ASYNC_JOB_NONE, !!migrateURI, - false, flags) < 0) + if (qemuProcessInit(driver, vm, QEMU_ASYNC_JOB_NONE, !!migrateURI, flags) < 0) goto cleanup; if (qemuProcessPrepareDomain(conn, driver, vm, flags) < 0) diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 13845d7..37081ad 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -68,6 +68,7 @@ typedef enum { VIR_QEMU_PROCESS_START_PAUSED = 1 << 1, VIR_QEMU_PROCESS_START_AUTODESTROY = 1 << 2, VIR_QEMU_PROCESS_START_PRETEND = 1 << 3, + VIR_QEMU_PROCESS_START_NEW = 1 << 4, /* internal, new VM is starting */ } qemuProcessStartFlags; int qemuProcessStart(virConnectPtr conn, @@ -93,7 +94,6 @@ int qemuProcessInit(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, bool migration, - bool snap, unsigned int flags); int qemuProcessPrepareDomain(virConnectPtr conn, -- 2.8.3

To avoid duplicating all the checks when starting a fresh VM from a possibly unchecked config, call the domain def validator. --- src/qemu/qemu_process.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f8afb36..068cb0e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4283,8 +4283,10 @@ qemuProcessStartWarnShmem(virDomainObjPtr vm) } static int -qemuProcessStartValidateXML(virDomainObjPtr vm, +qemuProcessStartValidateXML(virQEMUDriverPtr driver, + virDomainObjPtr vm, virQEMUCapsPtr qemuCaps, + virCapsPtr caps, unsigned int flags) { /* The bits we validate here are XML configs that we previously @@ -4301,9 +4303,14 @@ qemuProcessStartValidateXML(virDomainObjPtr vm, /* checks below should not be executed when starting a qemu process for a * VM that was running before (migration, snapshots, save). It's more * important to start such VM than keep the configuration clean */ - if ((flags & VIR_QEMU_PROCESS_START_NEW) && - virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) - return -1; + if ((flags & VIR_QEMU_PROCESS_START_NEW)) { + if (virDomainDefValidate(vm->def, caps, 0, driver->xmlopt) < 0) + return -1; + + if (virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) + return -1; + } + if (vm->def->mem.min_guarantee) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -4331,6 +4338,7 @@ static int qemuProcessStartValidate(virQEMUDriverPtr driver, virDomainObjPtr vm, virQEMUCapsPtr qemuCaps, + virCapsPtr caps, unsigned int flags) { size_t i; @@ -4358,7 +4366,7 @@ qemuProcessStartValidate(virQEMUDriverPtr driver, } - if (qemuProcessStartValidateXML(vm, qemuCaps, flags) < 0) + if (qemuProcessStartValidateXML(driver, vm, qemuCaps, caps, flags) < 0) return -1; VIR_DEBUG("Checking for any possible (non-fatal) issues"); @@ -4437,7 +4445,7 @@ qemuProcessInit(virQEMUDriverPtr driver, vm->def->os.machine))) goto cleanup; - if (qemuProcessStartValidate(driver, vm, priv->qemuCaps, flags) < 0) + if (qemuProcessStartValidate(driver, vm, priv->qemuCaps, caps, flags) < 0) goto cleanup; /* Do this upfront, so any part of the startup process can add -- 2.8.3

Since it will not be called from outside of conf we can unexport it too if we move it to the appropriate place. Test suite change is necessary since the error will be reported sooner now. --- src/conf/domain_conf.c | 45 ++++++++++++++++++++++++--------------------- src/conf/domain_conf.h | 6 ++---- src/libvirt_private.syms | 1 - src/qemu/qemu_process.c | 11 +++-------- tests/qemuxml2argvtest.c | 6 +++--- 5 files changed, 32 insertions(+), 37 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b8a6901..987b2fe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4580,8 +4580,29 @@ virDomainDefValidateDeviceIterator(virDomainDefPtr def, static int -virDomainDefValidateInternal(const virDomainDef *def ATTRIBUTE_UNUSED) +virDomainDefCheckDuplicateDiskInfo(const virDomainDef *def) { + size_t i; + size_t j; + + for (i = 0; i < def->ndisks; i++) { + for (j = i + 1; j < def->ndisks; j++) { + if (virDomainDiskDefCheckDuplicateInfo(def->disks[i], + def->disks[j]) < 0) + return -1; + } + } + + return 0; +} + + +static int +virDomainDefValidateInternal(const virDomainDef *def) +{ + if (virDomainDefCheckDuplicateDiskInfo(def) < 0) + return -1; + return 0; } @@ -24571,8 +24592,8 @@ virDomainDefNeedsPlacementAdvice(virDomainDefPtr def) int -virDomainDiskDefCheckDuplicateInfo(virDomainDiskDefPtr a, - virDomainDiskDefPtr b) +virDomainDiskDefCheckDuplicateInfo(const virDomainDiskDef *a, + const virDomainDiskDef *b) { if (STREQ(a->dst, b->dst)) { virReportError(VIR_ERR_XML_ERROR, @@ -24601,24 +24622,6 @@ virDomainDiskDefCheckDuplicateInfo(virDomainDiskDefPtr a, } -int -virDomainDefCheckDuplicateDiskInfo(virDomainDefPtr def) -{ - size_t i; - size_t j; - - for (i = 0; i < def->ndisks; i++) { - for (j = i + 1; j < def->ndisks; j++) { - if (virDomainDiskDefCheckDuplicateInfo(def->disks[i], - def->disks[j]) < 0) - return -1; - } - } - - return 0; -} - - /** * virDomainDefHasMemballoon: * @def: domain definition diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 215235f..5c900bb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3068,11 +3068,9 @@ virDomainParseMemory(const char *xpath, bool virDomainDefNeedsPlacementAdvice(virDomainDefPtr def) ATTRIBUTE_NONNULL(1); -int virDomainDiskDefCheckDuplicateInfo(virDomainDiskDefPtr a, - virDomainDiskDefPtr b) +int virDomainDiskDefCheckDuplicateInfo(const virDomainDiskDef *a, + const virDomainDiskDef *b) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int virDomainDefCheckDuplicateDiskInfo(virDomainDefPtr def) - ATTRIBUTE_NONNULL(1); int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, int maplen, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9b9bfab..02a957f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -207,7 +207,6 @@ virDomainDefAddController; virDomainDefAddImplicitDevices; virDomainDefAddUSBController; virDomainDefCheckABIStability; -virDomainDefCheckDuplicateDiskInfo; virDomainDefClearCCWAddresses; virDomainDefClearDeviceAliases; virDomainDefClearPCIAddresses; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 068cb0e..7fc4e80 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4303,14 +4303,9 @@ qemuProcessStartValidateXML(virQEMUDriverPtr driver, /* checks below should not be executed when starting a qemu process for a * VM that was running before (migration, snapshots, save). It's more * important to start such VM than keep the configuration clean */ - if ((flags & VIR_QEMU_PROCESS_START_NEW)) { - if (virDomainDefValidate(vm->def, caps, 0, driver->xmlopt) < 0) - return -1; - - if (virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) - return -1; - } - + if ((flags & VIR_QEMU_PROCESS_START_NEW) && + virDomainDefValidate(vm->def, caps, 0, driver->xmlopt) < 0) + return -1; if (vm->def->mem.min_guarantee) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index db42b7b..adec2f0 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -876,9 +876,9 @@ mymain(void) DO_TEST("disk-drive-discard", QEMU_CAPS_DRIVE_DISCARD); DO_TEST("disk-snapshot", NONE); - DO_TEST_FAILURE("disk-same-targets", - QEMU_CAPS_SCSI_LSI, - QEMU_CAPS_DEVICE_USB_STORAGE, QEMU_CAPS_NODEFCONFIG); + DO_TEST_PARSE_ERROR("disk-same-targets", + QEMU_CAPS_SCSI_LSI, + QEMU_CAPS_DEVICE_USB_STORAGE, QEMU_CAPS_NODEFCONFIG); DO_TEST("event_idx", QEMU_CAPS_VIRTIO_BLK_EVENT_IDX, QEMU_CAPS_VIRTIO_NET_EVENT_IDX, -- 2.8.3

Now with the proper domain config validation infrastructure the check can be moved to a place that doesn't make domains vanish. --- src/conf/domain_conf.c | 66 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 987b2fe..e21e566 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4148,20 +4148,6 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, } } - /* Validate LUN configuration */ - if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - /* volumes haven't been translated at this point, so accept them */ - if (!(disk->src->type == VIR_STORAGE_TYPE_BLOCK || - disk->src->type == VIR_STORAGE_TYPE_VOLUME || - (disk->src->type == VIR_STORAGE_TYPE_NETWORK && - disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk '%s' improperly configured for a " - "device='lun'"), disk->dst); - return -1; - } - } - if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0) return -1; @@ -4539,9 +4525,59 @@ virDomainDefPostParse(virDomainDefPtr def, static int -virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev ATTRIBUTE_UNUSED, +virDomainDiskDefValidate(const virDomainDiskDef *disk) +{ + /* Validate LUN configuration */ + if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + /* volumes haven't been translated at this point, so accept them */ + if (!(disk->src->type == VIR_STORAGE_TYPE_BLOCK || + disk->src->type == VIR_STORAGE_TYPE_VOLUME || + (disk->src->type == VIR_STORAGE_TYPE_NETWORK && + disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk '%s' improperly configured for a " + "device='lun'"), disk->dst); + return -1; + } + } + + return 0; +} + + +static int +virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, const virDomainDef *def ATTRIBUTE_UNUSED) { + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + return virDomainDiskDefValidate(dev->data.disk); + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_NET: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_HOSTDEV: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_LAST: + break; + } + return 0; } -- 2.8.3

Introduce a validation callback for qemu and move checking of min_guarantee to the new callback. --- src/qemu/qemu_domain.c | 17 +++++++++++++++++ src/qemu/qemu_process.c | 7 ------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fd6d6d2..7e64545 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2168,6 +2168,22 @@ qemuDomainDefPostParse(virDomainDefPtr def, return ret; } + +static int +qemuDomainDefValidate(const virDomainDef *def, + virCapsPtr caps ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (def->mem.min_guarantee) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Parameter 'min_guarantee' not supported by QEMU.")); + return -1; + } + + return 0; +} + + static const char * qemuDomainDefaultNetModel(const virDomainDef *def, virQEMUCapsPtr qemuCaps) @@ -2419,6 +2435,7 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = { .devicesPostParseCallback = qemuDomainDeviceDefPostParse, .domainPostParseCallback = qemuDomainDefPostParse, .assignAddressesCallback = qemuDomainDefAssignAddresses, + .domainValidateCallback = qemuDomainDefValidate, .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG | VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN }; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7fc4e80..1a9f176 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4307,13 +4307,6 @@ qemuProcessStartValidateXML(virQEMUDriverPtr driver, virDomainDefValidate(vm->def, caps, 0, driver->xmlopt) < 0) return -1; - if (vm->def->mem.min_guarantee) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Parameter 'min_guarantee' " - "not supported by QEMU.")); - return -1; - } - return 0; } -- 2.8.3