Re: [libvirt] [PATCH 4/3] qemu: don't leave vm in table on failure to attach

On 28.08.2013 22:56, Eric Blake wrote: > Failure to attach to a domain during 'virsh qemu-attach' left > the list of domains in an odd state: > > $ virsh qemu-attach 4176 > error: An error occurred, but the cause is unknown > > $ virsh list --all > Id Name State > ---------------------------------------------------- > 2 foo shut off >

> +++ b/src/qemu/qemu_driver.c > @@ -13623,8 +13623,11 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn, > > if (qemuProcessAttach(conn, driver, vm, pid, > pidfile, monConfig, monJSON) < 0) { > + if (qemuDomainObjEndJob(driver, vm) > 0) > + qemuDomainRemoveInactive(driver, vm); > + vm = NULL; > monConfig = NULL; > - goto endjob; > + goto cleanup; > } > While this part looks okay ...

> monConfig = NULL; > @@ -13632,7 +13635,6 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn, > dom = virGetDomain(conn, vm->def->name, vm->def->uuid); > if (dom) dom->id = vm->def->id; > > -endjob: > if (qemuDomainObjEndJob(driver, vm) == 0) { > vm = NULL; > goto cleanup; >

... I am not convinced about this part. I mean, in the context, we create Domain and store it into @dom. But if qemu process dies meanwhile, so the qemuDomainObjEndJob() returns zero, the @dom refers to a stale process. Or am I missing something here? I think these lines should be swapped.