Re: [libvirt] [PATCH] qemu: Always remove domain object if MigratePrepare fails

On 09/26/2011 05:50 AM, Jiri Denemark wrote: > If migration failed in Prepare phase after virDomainAssignDef and before > a job is started, the domain object was not properly removed. > --- > src/qemu/qemu_migration.c | 11 ++++++----- > 1 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > index 0a5a13d..ea49093 100644 > --- a/src/qemu/qemu_migration.c > +++ b/src/qemu/qemu_migration.c > @@ -1178,8 +1178,12 @@ cleanup: > virDomainDefFree(def); > VIR_FORCE_CLOSE(dataFD[0]); > VIR_FORCE_CLOSE(dataFD[1]); > - if (vm) > - virDomainObjUnlock(vm); > + if (vm) { > + if (vm->persistent) > + virDomainObjUnlock(vm); > + else > + qemuDomainRemoveInactive(driver, vm); I think this part is not quite right. This will also remove a transient destination vm on the success path. > + } > if (event) > qemuDomainEventQueue(driver, event); > qemuMigrationCookieFree(mig); > @@ -1188,9 +1192,6 @@ cleanup: > endjob: > if (qemuMigrationJobFinish(driver, vm) == 0) { > vm = NULL; > - } else if (!vm->persistent) { > - qemuDomainRemoveInactive(driver, vm); > - vm = NULL; > } I agree that moving this hunk out of endjob: and into cleanup: means that you will properly clean up for more failure cases, but I think you're missing a (ret < 0) check in cleanup: for this to be bullet-proof.