Re: [libvirt] [PATCH v2] qemu: neglect cur_balloon in supplied xml

cur_balloon value can change in between preparing external config and using it in operations like save and migrate. As a resutl operation will fail for ABI inconsistency. cur_balloon changes can not be predicted generally and thus operations will fail from time to time. Skip checking cur_balloon if domain lock can not be hold between preparing external config outside of libvirt and checking it against active config. Instead update cur_balloon value in external config from active config. This way it is protected from forges and is keeped up to date too. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; --- src/conf/domain_conf.c | 14 +++++++++++--- src/conf/domain_conf.h | 9 +++++++++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 29 ++++++++++++++++++++--------- src/qemu/qemu_domain.h | 6 +++--- src/qemu/qemu_driver.c | 5 +++-- src/qemu/qemu_migration.c | 4 ++-- 7 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 85f6e31..f1cf87f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18493,8 +18493,9 @@ virDomainDefVcpuCheckAbiStability(virDomainDefPtr src, * validation of custom XML config passed in during migration */ bool -virDomainDefCheckABIStability(virDomainDefPtr src, - virDomainDefPtr dst) +virDomainDefCheckABIStabilityFlags(virDomainDefPtr src, + virDomainDefPtr dst, + unsigned int flags) { size_t i; virErrorPtr err; @@ -18538,7 +18539,8 @@ virDomainDefCheckABIStability(virDomainDefPtr src, virDomainDefGetMemoryInitial(src)); goto error; } - if (src->mem.cur_balloon != dst->mem.cur_balloon) { + if (!(flags & VIR_DOMAIN_DEF_ABI_CHECK_SKIP_VOLATILE) && + src->mem.cur_balloon != dst->mem.cur_balloon) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain current memory %lld does not match source %lld"), dst->mem.cur_balloon, src->mem.cur_balloon); @@ -18963,6 +18965,12 @@ virDomainDefCheckABIStability(virDomainDefPtr src, return false; } +bool +virDomainDefCheckABIStability(virDomainDefPtr src, + virDomainDefPtr dst) +{ + return virDomainDefCheckABIStabilityFlags(src, dst, 0); +} static int virDomainDefAddDiskControllersForType(virDomainDefPtr def, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3792562..cd7b966 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2623,6 +2623,11 @@ typedef enum { VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST = 1 << 9, } virDomainDefFormatFlags; +typedef enum { + /* skip checking values like cur_balloon that can be changed meanwhile */ + VIR_DOMAIN_DEF_ABI_CHECK_SKIP_VOLATILE = 1 << 0, +} virDomainDefABICheckFlags; + virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr, const virDomainDef *def, virCapsPtr caps, @@ -2658,6 +2663,10 @@ virDomainObjPtr virDomainObjParseFile(const char *filename, bool virDomainDefCheckABIStability(virDomainDefPtr src, virDomainDefPtr dst); +bool virDomainDefCheckABIStabilityFlags(virDomainDefPtr src, + virDomainDefPtr dst, + unsigned int flags); + int virDomainDefAddImplicitDevices(virDomainDefPtr def); virDomainIOThreadIDDefPtr virDomainIOThreadIDFind(const virDomainDef *def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 42f664c..4e7840c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -208,6 +208,7 @@ virDomainDefAddController; virDomainDefAddImplicitDevices; virDomainDefAddUSBController; virDomainDefCheckABIStability; +virDomainDefCheckABIStabilityFlags; virDomainDefClearCCWAddresses; virDomainDefClearDeviceAliases; virDomainDefClearPCIAddresses; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d1f8175..4b45caf 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4620,21 +4620,32 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver, } -bool -qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, - virDomainDefPtr src, - virDomainDefPtr dst) +int +qemuDomainDefUpdateVolatile(virQEMUDriverPtr driver, + virDomainDefPtr src, + virDomainDefPtr dst) { virDomainDefPtr migratableDefSrc = NULL; virDomainDefPtr migratableDefDst = NULL; - const int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_UPDATE_CPU | VIR_DOMAIN_XML_MIGRATABLE; - bool ret = false; + const unsigned int copy_flags = VIR_DOMAIN_XML_SECURE | + VIR_DOMAIN_XML_UPDATE_CPU | + VIR_DOMAIN_XML_MIGRATABLE; + const unsigned int check_flags = VIR_DOMAIN_DEF_ABI_CHECK_SKIP_VOLATILE; + int ret = -1; - if (!(migratableDefSrc = qemuDomainDefCopy(driver, src, flags)) || - !(migratableDefDst = qemuDomainDefCopy(driver, dst, flags))) + + if (!(migratableDefSrc = qemuDomainDefCopy(driver, src, copy_flags)) || + !(migratableDefDst = qemuDomainDefCopy(driver, dst, copy_flags))) + goto cleanup; + + if (!virDomainDefCheckABIStabilityFlags(migratableDefSrc, + migratableDefDst, + check_flags)) goto cleanup; - ret = virDomainDefCheckABIStability(migratableDefSrc, migratableDefDst); + dst->mem.cur_balloon = src->mem.cur_balloon; + + ret = 0; cleanup: virDomainDefFree(migratableDefSrc); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2443e97..7581fff 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -578,9 +578,9 @@ int qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob); -bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, - virDomainDefPtr src, - virDomainDefPtr dst); +int qemuDomainDefUpdateVolatile(virQEMUDriverPtr driver, + virDomainDefPtr src, + virDomainDefPtr dst); bool qemuDomainAgentAvailable(virDomainObjPtr vm, bool reportError); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e70d3ce..a819f53 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3244,7 +3244,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) { goto endjob; } - if (!qemuDomainDefCheckABIStability(driver, vm->def, def)) { + if (qemuDomainDefUpdateVolatile(driver, vm->def, def) < 0) { virDomainDefFree(def); goto endjob; } @@ -15103,7 +15103,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, /* Transitions 5, 6, 8, 9 */ /* Check for ABI compatibility. We need to do this check against * the migratable XML or it will always fail otherwise */ - if (config && !qemuDomainDefCheckABIStability(driver, vm->def, config)) { + if (config + && qemuDomainDefUpdateVolatile(driver, vm->def, config) < 0) { virErrorPtr err = virGetLastError(); if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 89350c8..38f471b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3237,7 +3237,7 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver, VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) goto cleanup; - if (!qemuDomainDefCheckABIStability(driver, vm->def, def)) + if (qemuDomainDefUpdateVolatile(driver, vm->def, def) < 0) goto cleanup; rv = qemuDomainDefFormatLive(driver, def, false, true); @@ -3582,7 +3582,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, if (!newdef) goto cleanup; - if (!qemuDomainDefCheckABIStability(driver, *def, newdef)) { + if (qemuDomainDefUpdateVolatile(driver, *def, newdef) < 0) { virDomainDefFree(newdef); goto cleanup; }