Re: [libvirt] [PATCH v2 8/8] qemu: Fix some corner cases in persistent migration

When persistently migrating a domain to a destination host where the same domain already exists (i.e., it is persistent and shutdown at the destination), we would happily through away the original persistent

definition without properly freeing it. And when updating the definition fails for some reason we don't properly revert to the original state leaving the domain broken. In addition to fixing these issues, the patch also makes sure the domain definition parsed from a migration cookie is either used or freed. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - new patch src/qemu/qemu_migration.c | 56 +++++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c761657..1d556eb 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -525,6 +525,19 @@ qemuMigrationCookieAddPersistent(qemuMigrationCookiePtr mig, } +static virDomainDefPtr +qemuMigrationCookieGetPersistent(qemuMigrationCookiePtr mig) +{ + virDomainDefPtr def = mig->persistent; + + mig->persistent = NULL; + mig->flags &= ~QEMU_MIGRATION_COOKIE_PERSISTENT; + mig->flagsMandatory &= ~QEMU_MIGRATION_COOKIE_PERSISTENT; + + return def; +} + + static int qemuMigrationCookieAddNetwork(qemuMigrationCookiePtr mig, virQEMUDriverPtr driver, @@ -5554,47 +5567,51 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) static int qemuMigrationPersist(virQEMUDriverPtr driver, virDomainObjPtr vm, - qemuMigrationCookiePtr mig) + qemuMigrationCookiePtr mig, + bool ignoreSaveError) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virCapsPtr caps = NULL; virDomainDefPtr vmdef; + virDomainDefPtr oldDef = NULL; + unsigned int oldPersist = vm->persistent; virObjectEventPtr event; - bool newVM; int ret = -1; if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - newVM = !vm->persistent; vm->persistent = 1; + oldDef = vm->newDef; + vm->newDef = qemuMigrationCookieGetPersistent(mig); - if (mig->persistent) - vm->newDef = mig->persistent; + if (!(vmdef = virDomainObjGetPersistentDef(caps, driver->xmlopt, vm))) + goto error; - vmdef = virDomainObjGetPersistentDef(caps, driver->xmlopt, vm); - if (!vmdef) { - if (newVM) - vm->persistent = 0; - goto cleanup; - } - - if (virDomainSaveConfig(cfg->configDir, vmdef) < 0) - goto cleanup; + if (virDomainSaveConfig(cfg->configDir, vmdef) < 0 && !ignoreSaveError) + goto error; event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, - newVM ? - VIR_DOMAIN_EVENT_DEFINED_ADDED : - VIR_DOMAIN_EVENT_DEFINED_UPDATED); + oldPersist ? + VIR_DOMAIN_EVENT_DEFINED_UPDATED : + VIR_DOMAIN_EVENT_DEFINED_ADDED); qemuDomainEventQueue(driver, event); ret = 0; cleanup: + virDomainDefFree(oldDef); virObjectUnref(caps); virObjectUnref(cfg); return ret; + + error: + virDomainDefFree(vm->newDef); + vm->persistent = oldPersist;

+ vm->newDef = oldDef; + oldDef = NULL; + goto cleanup; } @@ -5653,7 +5670,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, */ if (flags & VIR_MIGRATE_OFFLINE) { if (retcode != 0 || - qemuMigrationPersist(driver, vm, mig) < 0) + qemuMigrationPersist(driver, vm, mig, false) < 0) goto endjob; dom = virGetDomain(dconn, vm->def->name, vm->def->uuid); @@ -5685,7 +5702,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, goto endjob; if (flags & VIR_MIGRATE_PERSIST_DEST) { - if (qemuMigrationPersist(driver, vm, mig) < 0) { + if (qemuMigrationPersist(driver, vm, mig, !v3proto) < 0) { /* Hmpf. Migration was successful, but making it persistent * was not. If we report successful, then when this domain * shuts down, management tools are in for a surprise. On the @@ -5796,6 +5813,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, qemuMonitorSetDomainLog(priv->mon, -1); VIR_FREE(priv->origname); virDomainObjEndAPI(&vm); + virDomainDefFree(qemuMigrationCookieGetPersistent(mig));

qemuMigrationCookieFree(mig); if (orig_err) { virSetError(orig_err);