ping
On 09.06.2016 17:32, Nikolay Shirokovskiy wrote:
> 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@xxxxxxxxxxxxx>
> ---
> 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;
> }
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list