On Mon, Sep 26, 2011 at 15:56:03 -0600, Eric Blake wrote:
> 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.
Oops, yes, I sent a v2 which fixes that. Thanks for the careful review.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list