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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list