On 09/04/2013 06:13 AM, Michal Privoznik wrote:
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 ...
This change exactly mirros what qemuDomainCreateXML was doing (as mentioned in the commit message)...
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;
...which left endjob as an unused label.
... 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.
If there is still a bug here, then it is pre-existing, and probably a similar bug in qemuDomainCreateXML. I'll take another look. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org