Re: [libvirt] [PATCH 3/6] conf: Introduce infrastructure to add config validation to define time

Recently there were a few patches adding checks to the post parse callback infrastructure that rejected previously accepted configuration. This unfortunately was not acceptable since we would fail to load the configs from the disk and the VMs would vanish. This patch adds helpers which are called in the appropriate places after the config is parsed when defining/starting a VM but not when reloading the configs. This infrastructure is meant for invalid configurations that were accepted previously and thus it's not executed when starting a VM from any saved state. Any checks that are necessary to be executed at that point still need to be added to qemuProcessValidateXML. --- src/conf/domain_conf.c | 67 ++++++++++++++++++++++++++++++++++++++---------- src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_domain.c | 21 +++++++++++++++ src/qemu/qemu_driver.c | 6 +++++ src/qemu/qemu_process.c | 22 +++++++++++----- 7 files changed, 100 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

+/** + * 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) +{ + size_t i; + + /* check configuration of individual devices */ + VIR_DOMAIN_DEF_VALIDATE_DEVICES(disk, virDomainDiskDefValidate); + + return 0; +} + +#undef VIR_DOMAIN_DEF_VALIDATE_DEVICES

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c @@ -5343,3 +5343,24 @@ qemuDomainDefValidateDiskLunSource(const virStorageSource *src) return 0; } + + +/** + * qemuDomainDefValidate: + * @def: domain definition + * + * Validates that the domain definition is valid for the qemu driver. This check + * is run when the domain is defined or started as new. This check is explicitly + * not executed when starting a VM from snapshot/saved state/migration to allow + * partially invalid configs to still run. + * + * Returns 0 on success, -1 on error. + */ +int +qemuDomainDefValidate(const virDomainDef *def) +{ + if (virDomainDefValidate(def) < 0) + return -1; + + return 0; +} diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f8dfc8..e6d22ec 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1799,6 +1799,9 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, if (virDomainCreateXMLEnsureACL(conn, def) < 0) goto cleanup; + if (qemuDomainDefValidate(def) < 0) + goto cleanup; + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; @@ -7275,6 +7278,9 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) goto cleanup; + if (qemuDomainDefValidate(def) < 0) + goto cleanup; + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c

@@ -4634,6 +4627,21 @@ qemuProcessStartValidateXML(virDomainObjPtr vm, return -1; } + /* 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 (!migration && !snapshot) { + if (qemuDomainDefValidate(vm->def) < 0) + return -1; + + if (virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) + return -1; + } + + /* checks below validate config that would make qemu fail to start */ + if (qemuValidateCpuCount(vm->def, qemuCaps) < 0) + return -1; + return 0; }